-
Notifications
You must be signed in to change notification settings - Fork 297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added enyo.dom.calcViewportPositionForNode helper function #185
Conversation
…ocating nodes within the viewport (to absolutely position a floating Popup near a particular DOM element, for instance) Enyo-DCO-1.1-Signed-off-by: Ian Beck <ian@onecrayon.com>
Any chance you want to write a test case for this to put into \enyo\tools\test\core\tests? Without it being used elsewhere in core, I'd be afraid it could break without us knowing it. |
…of whether body element is 100% height/width or has margins Added enyo.dom.getWindowHeight to compliment getWindowWidth Enyo-DCO-1.1-Signed-off-by: Ian Beck <ian@onecrayon.com>
…nyo.dom Enyo-DCO-1.1-Signed-off-by: Ian Beck <ian@onecrayon.com>
I totally spaced on the request for a test case for some reason, but now here it is! Good thing you had me write that, too, since it caused me to discover an oversight in the measurement logic for pages where the body is not 100% width/height without a scrollbar. :-) |
Yeah, I find writing tests helps me find corner cases a lot easier than just fixing a problem. |
Added enyo.dom.calcViewportPositionForNode helper function Reviewed-By: Ben Combee (ben.combee@palm.com)
//* Returns an object like `{top: 0, left: 0, bottom: 100, right: 100, height: 10, width: 10}` that represents the object's position within the viewport. Negative values mean part of the object is not visible. | ||
calcViewportPositionForNode: function(inNode) { | ||
// Parse upward and grab our positioning relative to the viewport | ||
var left = top = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just fixed a bug here in master, but wanted to point it out to you -- this doesn't declare a local variable named top, but instead uses a global named top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the FYI! I'm frankly unsure why I used that pattern at all; normally I separate everything into its own comma-delimited assignment. Good to know that the habit has a purpose behind it.
Here's my calcViewportPositionForNode addition to enyo.dom, as per the discussion here:
https://groups.google.com/d/topic/enyo-development/S-eY8HNLktU/discussion
Although my use-case for this is pretty specific to my particular app, the function seemed generically useful enough to be worth submitting to Enyo core for consideration.